@jmarshallnz : Please review. Perhaps there's a better way to handle (ab)uses like this? Perhaps revert SetCachedImage/GetCachedImage to being static again? Or use locks?

This doesn't look like it will do anything to address the problem. I think we need to serialize access to sqlite.

I'd serialise access to the shared database object (TextureDatabase) in the loader if we expect public functions to use private members of a threaded object.

I was going to serialize all sqlite DB calls. Given that we don't know which "mode" sqlite is compiled in (see http://www.sqlite.org/threadsafe.html ), I'm not sure what else to do. Had a brief discussion on IRC. Any other suggestions?

At the very least we're assuming multithread, right? So one presumes it can't be compiled singlethreaded, so you should be able to choose threadsafe at runtime. Other than mistakes such as this one, we don't (to my knowledge) use the same db object across threads.

I'm not really sure we can assume anything. Let me know if you think otherwise. On Linux it's just pulled in. The safest thing to do is assume it's compiled "singlethreaded" and account for it in our code. That's what I was going to do. Let me know if you think that makes sense.

If that were the case, how would we not be crashing all over the place? We use sqlite databases from multiple threads concurrently all the time. We just don't use shared instances (except in places where we have bugs such as this one).

I don't know but that sound convincing to me. If it's compiled for multi-threaded mode then each connection is supposed to be able to operate independently. I'll make that assumption and put a PR out there.

This should fix it (or at least come closer): #3000

Like @jmarshallnz said: we use concurrent db access in my other places too so those should trigger crashes then as well. My idea was that this specific crash was caused by accessing the texturedb-member in the m_thumbloader object from both the background-loader thread and GUIWindowPictures that's why I split those to (hopefully) fix this, which in a fact gives them both their own thumbloader-object with their own (pirvate) texturedb-member. @jmarshallnz : Any pointers on how to serialise this properly?

If PR3000 is taken then that should mean this one wont be needed since it (supposedly) solves the root problem rather than just this one.

@jimfcarroll : Of course, just wanted to make sure you understood the issue/fix in this PR correctly.

Ah thanks. I do now. :-)

from EricV in trac:

Comment(by EricV):
Tested the temporary fix and indeed I see no more crash. Thanks.

Haven't tested it myself. Hopefully i will get to it tonight.

I think it is better to do it like this anyway. IMO we shouldn't mix threaded and non-threaded uses of the same object: that's asking for trouble. Ideally we should perhaps even seperate the ThumbLoader class somehow so you don't even have the option to do so..

Confirmed working.

Merge this so it fixes the monthly build?

I'd say +1. Dunno what @jmarshallnz thinks about it...

I've merged this as it at least makes xbmc usable again. Perhaps there needs to be a more thorough fix but that will take some time.